Remove the transaction parameter from xenbus_switch_state and move the state

switch out of a transaction, in the few cases where it is inside one.

In order to behave properly, it is necessary for a driver to know its own
xenbus state (see changeset 9469:b3cb19d2b07f, for example).  This
value is stored as xenbus_device.state and updated by xenbus_switch_state.

If xenbus_switch_state occurs within a transaction, then there is a possibility
that the transaction would be aborted, leaving the state field dangerously out
of sync with the value currently in the store.

This fixes recent problems seen whereby bringing multiple devices up at the
same time results in some devices not coming up (often all of the even-numbered
ones, because of the pattern of transaction conflict).

Signed-off-by: Ewan Mellor <ewan@xensource.com>

diff --git a/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c
index 23fe51a36289c6913298b1ecef0822b244788e70..636caefdd8340377f4586863f2446c82360fb4fc 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c
+++ b/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c
@@ -142,7 +142,7 @@ static int blkback_probe(struct xenbus_device *dev,
        if (err)
                goto fail;
 
-       err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait);
+       err = xenbus_switch_state(dev, XenbusStateInitWait);
        if (err)
                goto fail;
 
@@ -270,7 +270,7 @@ static void frontend_changed(struct xenbus_device *dev,
                break;
 
        case XenbusStateClosing:
-               xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing);
+               xenbus_switch_state(dev, XenbusStateClosing);
                break;
 
        case XenbusStateClosed:
@@ -343,15 +343,17 @@ again:
                goto abort;
        }
 
-       err = xenbus_switch_state(dev, xbt, XenbusStateConnected);
-       if (err)
-               goto abort;
-
        err = xenbus_transaction_end(xbt, 0);
        if (err == -EAGAIN)
                goto again;
        if (err)
                xenbus_dev_fatal(dev, err, "ending transaction");
+
+       err = xenbus_switch_state(dev, XenbusStateConnected);
+       if (err)
+               xenbus_dev_fatal(dev, err, "switching to Connected state",
+                                dev->nodename);
+
        return;
  abort:
        xenbus_transaction_end(xbt, 1);

diff --git a/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c b/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c
index 6e29fa3d755b2560c46d41a6c6dfa4730daa0a28..49996362072b0a5919b4793ebfc403944529bc2f 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c
+++ b/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c
@@ -176,10 +176,6 @@ again:
                goto abort_transaction;
        }
 
-       err = xenbus_switch_state(dev, xbt, XenbusStateInitialised);
-       if (err)
-               goto abort_transaction;
-
        err = xenbus_transaction_end(xbt, 0);
        if (err) {
                if (err == -EAGAIN)
@@ -188,6 +184,8 @@ again:
                goto destroy_blkring;
        }
 
+       xenbus_switch_state(dev, XenbusStateInitialised);
+
        return 0;
 
  abort_transaction:
@@ -324,7 +322,7 @@ static void connect(struct blkfront_info *info)
                return;
        }
 
-       (void)xenbus_switch_state(info->xbdev, XBT_NULL, XenbusStateConnected);
+       (void)xenbus_switch_state(info->xbdev, XenbusStateConnected);
 
        /* Kick pending requests. */
        spin_lock_irq(&blkif_io_lock);
@@ -349,7 +347,7 @@ static void blkfront_closing(struct xenbus_device *dev)
 
        xlvbd_del(info);
 
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed);
+       xenbus_switch_state(dev, XenbusStateClosed);
 }
 
 
@@ -755,7 +753,7 @@ static void blkif_recover(struct blkfront_info *info)
 
        kfree(copy);
 
-       (void)xenbus_switch_state(info->xbdev, XBT_NULL, XenbusStateConnected);
+       (void)xenbus_switch_state(info->xbdev, XenbusStateConnected);
 
        /* Now safe for us to use the shared ring */
        spin_lock_irq(&blkif_io_lock);

diff --git a/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c
index 52a4fc98c8edccd86b7b28dece9b85141bd9d661..ea9a02e28e7acd8d4d53d8d96c8115ef8b5125f1 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c
@@ -92,7 +92,7 @@ static int netback_probe(struct xenbus_device *dev,
        if (err)
                goto fail;
 
-       err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait);
+       err = xenbus_switch_state(dev, XenbusStateInitWait);
        if (err) {
                goto fail;
        }
@@ -209,7 +209,7 @@ static void frontend_changed(struct xenbus_device *dev,
                break;
 
        case XenbusStateClosing:
-               xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing);
+               xenbus_switch_state(dev, XenbusStateClosing);
                break;
 
        case XenbusStateClosed:
@@ -254,7 +254,7 @@ static void connect(struct backend_info *be)
                return;
        }
 
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateConnected);
+       xenbus_switch_state(dev, XenbusStateConnected);
 }
 
 

diff --git a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c
index 220e9aa343a0ffd8778c50f03a150b8421d4bf1e..6c7b82f789ddc8065ebc96dbc8664f168133c06a 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c
+++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c
@@ -1216,7 +1216,7 @@ static void netfront_closing(struct xenbus_device *dev)
 
        close_netdev(info);
 
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed);
+       xenbus_switch_state(dev, XenbusStateClosed);
 }
 
 

diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c
index c64d2500671210de7ef5bc2b9740b25b2f2288e3..dc369d0efbb7ff1bd13f1b3a265852bfecc48670 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c
@@ -137,7 +137,7 @@ static int pciback_attach(struct pciback_device *pdev)
 
        dev_dbg(&pdev->xdev->dev, "Connecting...\n");
 
-       err = xenbus_switch_state(pdev->xdev, XBT_NULL, XenbusStateConnected);
+       err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
        if (err)
                xenbus_dev_fatal(pdev->xdev, err,
                                 "Error switching to connected state!");
@@ -165,7 +165,7 @@ static void pciback_frontend_changed(struct xenbus_device *xdev,
                break;
 
        case XenbusStateClosing:
-               xenbus_switch_state(xdev, XBT_NULL, XenbusStateClosing);
+               xenbus_switch_state(xdev, XenbusStateClosing);
                break;
 
        case XenbusStateClosed:
@@ -341,7 +341,7 @@ static int pciback_setup_backend(struct pciback_device *pdev)
                goto out;
        }
 
-       err = xenbus_switch_state(pdev->xdev, XBT_NULL, XenbusStateInitialised);
+       err = xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
        if (err)
                xenbus_dev_fatal(pdev->xdev, err,
                                 "Error switching to initialised state!");
@@ -386,7 +386,7 @@ static int pciback_xenbus_probe(struct xenbus_device *dev,
        }
 
        /* wait for xend to configure us */
-       err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait);
+       err = xenbus_switch_state(dev, XenbusStateInitWait);
        if (err)
                goto out;
 

diff --git a/linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c
index eed89eb5d3955f8bfbcd5b78c036e5003d5f2cda..ee154ba360572b2b3536eceacdbb61140636a238 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c
+++ b/linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c
@@ -96,10 +96,6 @@ static int pcifront_publish_info(struct pcifront_device *pdev)
        if (!err)
                err = xenbus_printf(trans, pdev->xdev->nodename,
                                    "magic", XEN_PCI_MAGIC);
-       if (!err)
-               err =
-                   xenbus_switch_state(pdev->xdev, trans,
-                                       XenbusStateInitialised);
 
        if (err) {
                xenbus_transaction_end(trans, 1);
@@ -118,6 +114,8 @@ static int pcifront_publish_info(struct pcifront_device *pdev)
                }
        }
 
+       xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
+
        dev_dbg(&pdev->xdev->dev, "publishing successful!\n");
 
       out:
@@ -186,7 +184,7 @@ static int pcifront_try_connect(struct pcifront_device *pdev)
                }
        }
 
-       err = xenbus_switch_state(pdev->xdev, XBT_NULL, XenbusStateConnected);
+       err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
        if (err)
                goto out;
 
@@ -205,8 +203,7 @@ static int pcifront_try_disconnect(struct pcifront_device *pdev)
        prev_state = xenbus_read_driver_state(pdev->xdev->nodename);
 
        if (prev_state < XenbusStateClosing)
-               err = xenbus_switch_state(pdev->xdev, XBT_NULL,
-                                       XenbusStateClosing);
+               err = xenbus_switch_state(pdev->xdev, XenbusStateClosing);
 
        if (!err && prev_state == XenbusStateConnected)
                pcifront_disconnect(pdev);

diff --git a/linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c
index 6ce5f0789144088e05044189ef2fd04833140273..14d6feb75ee1d2befcf88c6f4f440f5bfb187429 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c
+++ b/linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c
@@ -87,7 +87,7 @@ static int tpmback_probe(struct xenbus_device *dev,
                goto fail;
        }
 
-       err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait);
+       err = xenbus_switch_state(dev, XenbusStateInitWait);
        if (err) {
                goto fail;
        }
@@ -175,7 +175,7 @@ static void frontend_changed(struct xenbus_device *dev,
                break;
 
        case XenbusStateClosing:
-               xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing);
+               xenbus_switch_state(dev, XenbusStateClosing);
                break;
 
        case XenbusStateClosed:
@@ -247,18 +247,15 @@ again:
                goto abort;
        }
 
-       err = xenbus_switch_state(dev, xbt, XenbusStateConnected);
-       if (err)
-               goto abort;
-
-       be->tpmif->status = CONNECTED;
-
        err = xenbus_transaction_end(xbt, 0);
        if (err == -EAGAIN)
                goto again;
-       if (err) {
+       if (err)
                xenbus_dev_fatal(be->dev, err, "end of transaction");
-       }
+
+       err = xenbus_switch_state(dev, XenbusStateConnected);
+       if (!err)
+               be->tpmif->status = CONNECTED;
        return;
 abort:
        xenbus_transaction_end(xbt, 1);

diff --git a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c
index 8126705aeb0b48cf695cbf4447fb8c6f81fbdb85..38ac7f2c49b8244c43a117371e83e4fd4552a661 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c
@@ -84,9 +84,7 @@ int xenbus_watch_path2(struct xenbus_device *dev, const char *path,
 EXPORT_SYMBOL_GPL(xenbus_watch_path2);
 
 
-int xenbus_switch_state(struct xenbus_device *dev,
-                       xenbus_transaction_t xbt,
-                       XenbusState state)
+int xenbus_switch_state(struct xenbus_device *dev, XenbusState state)
 {
        /* We check whether the state is currently set to the given value, and
           if not, then the state is set.  We don't want to unconditionally
@@ -94,6 +92,12 @@ int xenbus_switch_state(struct xenbus_device *dev,
           unnecessarily.  Furthermore, if the node has gone, we don't write
           to it, as the device will be tearing down, and we don't want to
           resurrect that directory.
+
+          Note that, because of this cached value of our state, this function
+          will not work inside a Xenstore transaction (something it was
+          trying to in the past) because dev->state would not get reset if
+          the transaction was aborted.
+
         */
 
        int current_state;
@@ -102,12 +106,12 @@ int xenbus_switch_state(struct xenbus_device *dev,
        if (state == dev->state)
                return 0;
 
-       err = xenbus_scanf(xbt, dev->nodename, "state", "%d",
-                              &current_state);
+       err = xenbus_scanf(XBT_NULL, dev->nodename, "state", "%d",
+                          &current_state);
        if (err != 1)
                return 0;
 
-       err = xenbus_printf(xbt, dev->nodename, "state", "%d", state);
+       err = xenbus_printf(XBT_NULL, dev->nodename, "state", "%d", state);
        if (err) {
                if (state != XenbusStateClosing) /* Avoid looping */
                        xenbus_dev_fatal(dev, err, "writing new state");
@@ -193,7 +197,7 @@ void xenbus_dev_fatal(struct xenbus_device *dev, int err, const char *fmt,
        _dev_error(dev, err, fmt, ap);
        va_end(ap);
 
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing);
+       xenbus_switch_state(dev, XenbusStateClosing);
 }
 EXPORT_SYMBOL_GPL(xenbus_dev_fatal);
 

diff --git a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c
index 61158a60348edac56c08511220a400b67482f44f..d3f37e636d595d4d52a7982bfaf6864e771dd3d3 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c
@@ -364,7 +364,7 @@ static int xenbus_dev_probe(struct device *_dev)
        return 0;
 fail:
        xenbus_dev_error(dev, err, "xenbus_dev_probe on %s", dev->nodename);
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed);
+       xenbus_switch_state(dev, XenbusStateClosed);
        return -ENODEV;
 }
 
@@ -381,7 +381,7 @@ static int xenbus_dev_remove(struct device *_dev)
        if (drv->remove)
                drv->remove(dev);
 
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed);
+       xenbus_switch_state(dev, XenbusStateClosed);
        return 0;
 }
 

diff --git a/linux-2.6-xen-sparse/include/xen/xenbus.h b/linux-2.6-xen-sparse/include/xen/xenbus.h
index 6d69963870b52c87fdb186ea26e33267f6c18ea8..b5a9da5f64f1aec41c95bdaf8f04a2c154c94827 100644 (file)
--- a/linux-2.6-xen-sparse/include/xen/xenbus.h
+++ b/linux-2.6-xen-sparse/include/xen/xenbus.h
@@ -204,14 +204,10 @@ int xenbus_watch_path2(struct xenbus_device *dev, const char *path,
 
 /**
  * Advertise in the store a change of the given driver to the given new_state.
- * Perform the change inside the given transaction xbt.  xbt may be NULL, in
- * which case this is performed inside its own transaction.  Return 0 on
- * success, or -errno on error.  On error, the device will switch to
- * XenbusStateClosing, and the error will be saved in the store.
+ * Return 0 on success, or -errno on error.  On error, the device will switch
+ * to XenbusStateClosing, and the error will be saved in the store.
  */
-int xenbus_switch_state(struct xenbus_device *dev,
-                       xenbus_transaction_t xbt,
-                       XenbusState new_state);
+int xenbus_switch_state(struct xenbus_device *dev, XenbusState new_state);
 
 
 /**